Add more information on _failed_to_convert_ exception (#21946)#27034
Add more information on _failed_to_convert_ exception (#21946)#27034cbuescher merged 1 commit intoelastic:masterfrom
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
@elasticmachine please test this |
There was a problem hiding this comment.
Is it possible to not implement BytesReference just to throw the exception but to actually use a "source" that will trigger some JsonParseException when creating the parser from source? I think that would be more realistic and a better test.
|
@cbuescher Make sense! Thanks a lot! 😃 |
There was a problem hiding this comment.
Looking at the issue this is trying to fix (#21946), I'm not sure if we need to include the "reformat" and the "x_content_type" in the slow log output.
There was a problem hiding this comment.
I pushed 96b88d46cf59a08c9b69e8433364d0c5abc04d30 . Thanks very much! 👍
|
@elasticmachine test this please |
|
@ppf2 Cloud you take a look at this? 😄 |
There was a problem hiding this comment.
Can you change this assertion to match the whole error string? It might be a bit fragile to future changes in tests, but since we don't randomize anything here the error should stay stable for a bit and this makes it easiert to understand the whole error structure from reading the test.
There was a problem hiding this comment.
Thanks! I pushed 9153b6d99567d6b1b68160d739b2656ddf537c74 . We cannot put the while error message here because the internal class org.elasticsearch.common.bytes.BytesReference$MarkSupportingStreamInputWrapper does not implement a toString() so the results of toString() are different each time.
There was a problem hiding this comment.
Sorry, what I meant by the previous request was to do an assertion on the whole error string (e.g. wie assertEquals), unless there are any reasons preventing this.
There was a problem hiding this comment.
Right, I didn't see your comment above. Makes sense to me now, sorry for the noise.
cbuescher
left a comment
There was a problem hiding this comment.
LGTM, I will kick of the CI tests
|
@liketic CI doesn't seem happy, although the error doesn't seem to be related to your changes, could you either rebase your branch or merge in master before I kick off another build? |
|
@cbuescher Rebase done. Please try again. |
|
@elasticmachine test this please |
* 6.x: test: Break apart the multi type aspect of rolling upgrade tests, Upgrade to Jackson 2.8.10 (#27230) [Docs] Fix minor paragraph indentation error for multiple Indices params (#25535) Fix inconsistencies in the rest api specs for `tasks` (#27163) Adjust RestHighLevelClient method modifiers (#27238) Add more information on `_failed_to_convert_` exception (#27034) Remove ElasticsearchQueryCachingPolicy (#27190) Backport the size-based index rollver to v6.1.0 Add size-based condition to the index rollover API (#27160) Remove the single argument Environment constructor (#27235) Fix RestGetAction name typo
* master: (25 commits) Disable bwc tests in preparation of backporting #26931 TemplateUpgradeService should only run on the master (#27294) Die with dignity while merging Fix profiling naming issues (#27133) Correctly encode warning headers Fixed references to Multi Index Syntax (#27283) Add an active Elasticsearch WordPress plugin link (#27279) Setting url parts as required to reflect the code base (#27263) keys in aggs percentiles need to be in quotes. (#26905) Align routing param type with search.json (#26958) Update to support bulk updates by query (#27172) Remove duplicated SnapshotStatus (#27276) add split index reference in indices.asciidoc Add ability to split shards (#26931) [Docs] Fix minor paragraph indentation error for multiple Indices params (#25535) Upgrade to Jackson 2.8.10 (#27230) Fix inconsistencies in the rest api specs for `tasks` (#27163) Adjust RestHighLevelClient method modifiers (#27238) Remove unused parameters in AnalysisRegistry (#27232) Add more information on `_failed_to_convert_` exception (#27034) ...
* ccr: (127 commits) Disable bwc tests in preparation of backporting elastic#26931 TemplateUpgradeService should only run on the master (elastic#27294) Die with dignity while merging Fix profiling naming issues (elastic#27133) Correctly encode warning headers Fixed references to Multi Index Syntax (elastic#27283) Add an active Elasticsearch WordPress plugin link (elastic#27279) Setting url parts as required to reflect the code base (elastic#27263) keys in aggs percentiles need to be in quotes. (elastic#26905) Align routing param type with search.json (elastic#26958) Update to support bulk updates by query (elastic#27172) Remove duplicated SnapshotStatus (elastic#27276) add split index reference in indices.asciidoc Add ability to split shards (elastic#26931) [Docs] Fix minor paragraph indentation error for multiple Indices params (elastic#25535) Upgrade to Jackson 2.8.10 (elastic#27230) Fix inconsistencies in the rest api specs for `tasks` (elastic#27163) Adjust RestHighLevelClient method modifiers (elastic#27238) Remove unused parameters in AnalysisRegistry (elastic#27232) Add more information on `_failed_to_convert_` exception (elastic#27034) ...
Close #21946